feat: Add optional floor value to AGC and performance optimizations#808
feat: Add optional floor value to AGC and performance optimizations#808yara-blue merged 4 commits intoRustAudio:masterfrom
Conversation
- Allows `AGC` to have a configurable enforced minimum value
becb3d1 to
c8db399
Compare
Should we also remove most of the docs for this and add links to the struct and the examples? Since i don't think we need this anymore |
|
Nice to see your active maintenance on this one! As you're updating anyway, may I put in a couple of thoughts as well?
And three questions:
|
I would keep the docs on the source member and link from the struct actually. Since users will generally use AGC like this: let stream = stream
.possibly_disconnected_channels_to_mono()
.constant_samplerate(SAMPLE_RATE)
.limit(LimitSettings::live_performance())
.process_buffer::<BUFFER_SIZE, _>(move |buffer| {
<secret things> // not really look at zed/crates/audio/src/audio.rs if your curious
})
.denoise()
.context("Could not set up denoiser")?
.automatic_gain_control(0.90, 1.0, 0.0, 5.0)
.periodic_access(Duration::from_millis(100), move |agc_source| {
agc_source
.set_enabled(LIVE_SETTINGS.auto_microphone_volume.load(Ordering::Relaxed));
let denoise = agc_source.inner_mut();
denoise.set_enabled(LIVE_SETTINGS.denoise.load(Ordering::Relaxed));
}); |
I really really do not want to expand rodio's API/patterns if we can. As I am actively working on rewriting/re-architecturing the audio pipeline. The larger our API the harder it well get to re-do it. Right now we only really support periodic access as parameter change method. If we stabilize the use of atomics/handles as is experimental here it will become far harder to make the changes I need to make. |
Performance improvements: - Cache atomic loads at start of `process_sample` to reduce redundant operations - Change floor from `Option<f32>` to `f32`, unwrap once in setter instead of every sample - Pass cached values as parameters to helper methods to avoid redundant atomic loads Algorithm fixes: - Fix `update_peak_level` to use instant attack `(0.0)` instead of `attack_coeff` - Remove unused `min_attack_coeff` logic that was always 0.0 by default and caused errors when not API improvements: - Move `duration_to_coefficient` to `math.rs` with `pub(crate)` visibility for reuse - Update all examples and benchmarks to use `AutomaticGainControlSettings::default()` - Increase default `absolute_max_gain` from `5.0` to `7.0` Documentation: - Fix incorrect comments in update_peak_level, set_floor, and inner methods - Clarify instant attack behavior in peak detection
:)
I agree… Done.
Not very correct I looked more into it, we don't need a We already have: let attack_speed = if desired_gain > self.current_gain {
attack_coeff
} else {
release_coeff
};To do this so peak doing the same thing for
Maybe we should work together on the defaults because for me, it's working fine, see attached video: asdasdasd.mp4But your issue with
@dvdsk responded to this, I don't feel that I have sufficient knowledge on the current state to provide valuable input on that. Added some optimisations for AGC: Increased default 1. Cached Atomic Loads
2. Optimized Floor Storage
3. Modified Helper Methods with Parameters
|
|
Good to merge if you ask me! 🎉 |
This PR introduces an optional floor value for the AGC, enabling users to set a minimum output level that prevents the gain from dropping below a specified threshold.
For example, setting a floor value of
1.0ensures that the audio output is always at or above the original source level, preventing excessive attenuation.It also updates the example AGC to use the new defaults, I noticed forgot to do that in #759